-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implementing python's global
and nonlocal
#1735
Conversation
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/scopes/Scope.kt
Outdated
Show resolved
Hide resolved
global
global
and nonlocal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd appreciate to have two PRs:
- one for the CPG changes (with it's own tests, so that we can see how it's supposed to work in general and not only in the specific python use-case)
- one making use of the first PR by implementing the Python stuff
I get why you modify the scope of a Reference
as this is how the Python front-end works. However, don't we need a scope modifier for the declaration (I get that this will be problematic with the Python "add declarations pass"?
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/StatementBuilder.kt
Outdated
Show resolved
Hide resolved
...est/kotlin/de/fraunhofer/aisec/cpg/frontends/python/statementHandler/StatementHandlerTest.kt
Show resolved
Hide resolved
Done. #1742
The declaration's scope is not modified. Only the "lookup scope" of the symbol that you are trying to resolve in the current scope is modified (e.g., from the local to the global scope). The trick is that the add-declaration-pass will then create the variable in the lookup-scope (if it does not exist). So if we have a |
This PR adds a new node type `LookupScopeStatement`, which can be used to adjust the lookup scope of symbols that are resolved in the current scope. Most prominent examples are Python's `global` and `nonlocal` statements. Support for python will be added in a later PR. This will only add the node and provides the basic functionality in the lookup.
This PR tries to implement python's `global` keyword and tries to replicate python variable resolution as good as possible.
7820a1b
to
4d793e8
Compare
Quality Gate passedIssues Measures |
...language-python/src/main/kotlin/de/fraunhofer/aisec/cpg/frontends/python/StatementHandler.kt
Show resolved
Hide resolved
...language-python/src/main/kotlin/de/fraunhofer/aisec/cpg/frontends/python/StatementHandler.kt
Show resolved
Hide resolved
...language-python/src/main/kotlin/de/fraunhofer/aisec/cpg/frontends/python/StatementHandler.kt
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
☔ View full report in Codecov by Sentry. |
This PR tries to implement python's
global
keyword and tries to replicate python variable resolution as good as possible.I had to add a
ReferenceScopeModifierStatement
node, which is able to solve bothglobal
andnonlocal
, but I am not 100 % happy with the solution. Another approach would be to store this scope modification in the scope itself, so this PR would need some discussion.nonlocal
cannot be tested until we actually support parsing nested functions #1736